Skip to content

Add Starred API, revised Watchers API and Improved Docs #197

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Sep 23, 2014
Merged

Add Starred API, revised Watchers API and Improved Docs #197

merged 12 commits into from
Sep 23, 2014

Conversation

valtlfelipe
Copy link
Contributor

  • Added Starred API for get starred repos, star an repo and unstar an repo for authenticated users.
  • Revised Watchers API using a new endpoint as shown in the GitHub API.
  • Improved Documentation | Added incomplete Activity documentation for Starred and Watchers API.

return $this->get('user/starred', array(
'page' => $page
));
return new Starred($this->client);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing the return value of an existing method is a BC break

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true, i will fix this

@stof
Copy link
Contributor

stof commented Sep 17, 2014

Fixing the watchers API is already covered in #183

@valtlfelipe
Copy link
Contributor Author

I just commited restoring back the method and creating a new one so it does not break the usage.



/**
* @Deprecated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should write it as @deprecated Use watchers() instead

@valtlfelipe
Copy link
Contributor Author

I just commited removing the unused class (I forgot to remove it from git) and updated the docblock's.



/**
* @Deprecated Use watchers() instead
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you use same lowercase as other docblocks?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, sorry about this. I saw you already fixed it! Thanks ;)

cursedcoder added a commit that referenced this pull request Sep 23, 2014
Add Starred API, revised Watchers API and Improved Docs
@cursedcoder cursedcoder merged commit 75941c1 into KnpLabs:master Sep 23, 2014
@cursedcoder
Copy link
Contributor

good job

@valtlfelipe
Copy link
Contributor Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants